Skip to content

Conversation

@cowwoc
Copy link
Contributor

@cowwoc cowwoc commented Oct 12, 2025

Problem

Maven 4 automatically injects --module-version ${project.version} into compiler arguments during execution, but the build cache validates properties before execution. This creates a timing mismatch that causes cache invalidation:

  • First build: No cache exists → properties captured during/after execution (WITH Maven 4 injection)
  • Second build: Cache exists → properties captured during validation (WITHOUT injection yet) → mismatch → cache invalidated

Root Cause Analysis

The cache extension currently reads plugin properties at different lifecycle points for different builds:

First Build Timeline:
  1. findCachedBuild() → no cache found
  2. Mojos execute → Maven 4 injects --module-version
  3. beforeMojoExecution() fires → captures properties WITH injection
  4. save() stores properties from execution events

Second Build Timeline:
  1. findCachedBuild() → cache found
  2. verifyCacheConsistency() → creates mojo via getConfiguredMojo()
  3. Reads properties → WITHOUT injection (too early in lifecycle)
  4. Compares to first build's stored values (WITH injection)
  5. MISMATCH → cache invalidated!

The problem: validation reads BEFORE injection, storage reads AFTER injection.

Solution

Capture properties at validation time for ALL builds (even when no cache exists). Store ONLY validation-time values in the cache. This ensures both validation and storage read at the same lifecycle point.

Implementation

  1. Modified CacheResult to store validation-time mojo events
  2. Modified BuildCacheMojosExecutionStrategy to capture properties immediately after findCachedBuild()
  3. Modified save() to store ONLY validation-time events (fails with AssertionError if missing)

Key Code Changes

// In BuildCacheMojosExecutionStrategy.execute():
result = cacheController.findCachedBuild(session, project, mojoExecutions, skipCache);

// NEW: Always capture validation-time properties when cache is enabled
if (cacheState == INITIALIZED) {
    Map<String, MojoExecutionEvent> validationTimeEvents =
        captureValidationTimeProperties(session, project, mojoExecutions);
    result = CacheResult.rebuilded(result, validationTimeEvents);
}
// In save():
// Validation-time events MUST exist - no fallback to execution-time
if (result.getValidationTimeEvents() == null || result.getValidationTimeEvents().isEmpty()) {
    throw new AssertionError("Validation-time properties not captured - this is a bug");
}
cacheController.save(result, mojoExecutions, result.getValidationTimeEvents());

Note: Execution-time values are still captured by MojoParametersListener for logging/debugging during the build, but are NOT stored in the cache.

New Timeline (Both Builds)

First Build:
  1. findCachedBuild() → no cache
  2. captureValidationTimeProperties() → reads WITHOUT injection
  3. Mojos execute → Maven 4 injects (logged but not stored)
  4. save() stores ONLY validation-time properties → WITHOUT injection

Second Build:
  1. findCachedBuild() → cache found
  2. verifyCacheConsistency() → reads WITHOUT injection
  3. captureValidationTimeProperties() → reads WITHOUT injection
  4. Compares to first build → BOTH without injection → MATCH!

What Gets Stored vs. Logged

Data Stored in Cache Available in Logs
Validation-time properties (no injection) ✅ Yes ✅ Yes
Execution-time properties (with injection) ❌ No ✅ Yes

This approach ensures cache consistency while preserving debugging information in build logs.

Benefits Over PR #391

Aspect PR #391 (ignorePattern) This PR (Validation-Time Capture)
Approach Workaround: Filter mismatched values Fix: Eliminate timing mismatch
Configuration Required None needed
Flexibility Pattern must match version format Format-agnostic
Maintenance Patterns need updates No maintenance
Scope Only fixes known patterns Fixes ALL auto-injected properties

Testing

Created comprehensive integration tests:

  1. Maven4JpmsModuleTest - JPMS module with Maven 4 auto-injection of --module-version
  2. ExplicitModuleVersionTest - JPMS module with explicit moduleVersion configuration
  3. NonJpmsProjectTest - Regular Java project without JPMS (ensures no regression)
  4. MultiModuleJpmsTest - Multi-module project with mixed JPMS and non-JPMS modules

All tests verify:

  • ✅ First build creates cache
  • ✅ Second build restores from cache (NO cache invalidation)
  • ✅ NO ignorePattern configuration required

Backward Compatibility

  • Fully backward compatible
  • Existing caches remain valid
  • No configuration changes required
  • ignorePattern still works but is no longer necessary for Maven 4 injection

Alternative Approaches Considered

  1. Delay validation until execution ❌ - Would lose early cache-miss detection performance benefit
  2. Pattern-based filtering (PR Add ignorePattern attribute to TrackedProperty for array filtering #391) ❌ - Treats symptom (value mismatch) instead of root cause (timing mismatch)
  3. Maven core changes ❌ - Outside our control, would take years to deploy
  4. Store both validation-time and execution-time ❌ - Unnecessary complexity, doesn't solve the consistency problem
  5. Validation-time capture + storage (this PR) ✅ - Fixes root cause, no configuration needed, preserves logging for debugging

Related Issues

Migration

No migration needed. This change is transparent to users. Existing projects will benefit automatically.

@cowwoc cowwoc force-pushed the fix-375-validation-time-capture branch 3 times, most recently from 57a76d8 to d2c8071 Compare October 12, 2025 07:41
Copy link
Contributor

@elharo elharo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to run mvn spotless:apply

cowwoc added a commit to cowwoc/maven-build-cache-extension that referenced this pull request Oct 29, 2025
- Upgrade spotless-maven-plugin from 2.44.5 to 3.0.0
- Upgrade palantir-java-format from 2.56.0 to 2.81.0 for Java 25 support
- Run mvn spotless:apply to format code

Addresses review feedback on PR apache#395.
@cowwoc
Copy link
Contributor Author

cowwoc commented Oct 29, 2025

@elharo Done. Try now.

@cowwoc cowwoc requested a review from elharo October 29, 2025 18:05
cowwoc added a commit to cowwoc/maven-build-cache-extension that referenced this pull request Dec 1, 2025
- Split run-on sentence in AssertionError message
- Use HashMap import instead of fully qualified name
- Move Mojo variable declaration into try block (nested try-finally)
- Return defensive copy from getValidationTimeEvents() for encapsulation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
cowwoc added a commit to cowwoc/maven-build-cache-extension that referenced this pull request Dec 1, 2025
- Split run-on sentence in AssertionError message
- Use HashMap import instead of fully qualified name
- Move Mojo variable declaration into try block (nested try-finally)
- Return defensive copy from getValidationTimeEvents() for encapsulation
@cowwoc cowwoc force-pushed the fix-375-validation-time-capture branch from 43445c6 to fda621a Compare December 1, 2025 18:05
cowwoc added a commit to cowwoc/maven-build-cache-extension that referenced this pull request Dec 1, 2025
- Split run-on sentence in AssertionError message
- Use HashMap import instead of fully qualified name
- Move Mojo variable declaration into try block (nested try-finally)
- Make getValidationTimeEvents() package-private
@cowwoc cowwoc force-pushed the fix-375-validation-time-capture branch from fda621a to 6cf3a36 Compare December 1, 2025 18:09
@cowwoc cowwoc requested a review from elharo December 1, 2025 18:10
…nate timing mismatch

Maven 4 automatically injects --module-version into compiler arguments during
execution, but cache validation happens before execution. This creates a timing
mismatch that causes cache invalidation.

This fix captures properties at validation time for ALL builds and stores ONLY
validation-time values in the cache, ensuring consistent reading at the same
lifecycle point.

Changes:
- Modified CacheResult to store validation-time mojo events
- Added captureValidationTimeProperties() to BuildCacheMojosExecutionStrategy
- Modified execute() to capture properties after findCachedBuild()
- Modified save() to store ONLY validation-time events (AssertionError if missing)
- Added comprehensive integration tests (Maven4Jpms, ExplicitModuleVersion, NonJpms, MultiModule)

Benefits:
- No configuration required (vs ignorePattern in PR apache#391)
- Fixes root cause (timing mismatch) not symptom (value mismatch)
- Works for ALL Maven 4 auto-injected properties
- Preserves execution-time logging for debugging

Alternative to PR apache#391
- Upgrade spotless-maven-plugin from 2.44.5 to 3.0.0
- Upgrade palantir-java-format from 2.56.0 to 2.81.0 for Java 25 support
- Run mvn spotless:apply to format code

Addresses review feedback on PR apache#395.
- Split run-on sentence in AssertionError message
- Use HashMap import instead of fully qualified name
- Move Mojo variable declaration into try block (nested try-finally)
- Make getValidationTimeEvents() package-private
@cowwoc cowwoc force-pushed the fix-375-validation-time-capture branch from 6cf3a36 to 04b6246 Compare December 1, 2025 18:50
@cowwoc cowwoc requested a review from elharo December 1, 2025 19:25
- Fix invalid XML in pom.xml comment (-- not allowed in XML comments)
- Fix cache config XML validation errors by simplifying config
- Change tests to use 'package' goal instead of 'compile' since cache
  extension stores artifacts, not intermediate output directories
- Fix AssertionError when running clean-only builds by gracefully
  skipping cache storage when no cacheable mojos executed
@cowwoc
Copy link
Contributor Author

cowwoc commented Dec 1, 2025

@elharo I've pushed a commit that should fix the latest build failures. Try now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants